-
Notifications
You must be signed in to change notification settings - Fork 644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #1635 - Escape underscores in packages for markdown #2243
Conversation
Oh, cool! There was also #1979, which was closed because we called it out of scope. But then there's also #1539 too, which is still open. The underlying issue for #1979 and #1539 is that we always send Markdown-rendered emails, but some users can only receive plain text. And yes, they've reported it to us as a bug. The fix you've made here will arguably make plain text messages a bit worse. Would you be open to augmenting this fix to also do the following?
This would probably require ditching MarkdownMailer, but that's fine with us. |
Sure - as far as I know the MarkdownMailer converts the markdown to html, right? So I would remove the MarkdownMailer and use another component for Markdown to HTML translation and also provide a pure Plain Text mail and send both as multipart message, right? Just to make sure I understand everything. Is there currently a Markdown renderer included in the project? If not: Which one do you prefer? |
I think you are correct. MarkdownSharp is probably what we would prefer for now, as it's what MarkdownMailer used. Thanks! |
Moved MailSender stuff from the Markdown Mailer into the project.
I got some time to work an this issue. Hope I find some more time on the weekend for the desired functionality. |
Sorry for being late. Current situation: I removed the MarkdownMailer Package, but I included the Source Code from @half-ogre. Idea: And then add the needed pure PlainText mailings, which can be totally different to the markdown => html mailing I'm not an email expert, but currently there should already be a plain text mailing - the pure text in markdown syntax should be there as plaintext:
Should I continue and add the plaintext parameter? |
@robertmuehsig, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
Is this pull request still interesting? |
@xavierdecoster can you please review? |
Bump! |
@qianjun22 this is an ancient PR, can we please figure out if we want it and merge ? |
Closing this PR since it's very old and has a lot of merge conflicts. |
Is this something that is still a problem? I could create a new PR. |
This: #1635 is still open, and I couldn't find any indication in the code that it was.. a new PR will be great. |
Should fix this issue #1635
I tested it with this package https://www.nuget.org/packages/JQueryFileUpload_Demo_with_Backload/
Now he ReportPackageRequest creates such an MailBody with correct escapted underscore. Poor mans solution, but should work.
Email: admin (...)
Package: JQueryFileUpload_Demo_with_Backload
http://nuget.localtest.me/packages/JQueryFileUpload\_Demo\_with\_Backload/
Version: 1.9.2.4
http://nuget.localtest.me/packages/JQueryFileUpload\_Demo\_with\_Backload/1.9.2.4
User: admin (...)
http://nuget.localtest.me/profiles/admin
Reason:
The package was published as the wrong version
Message:
test
Message sent from NuGet Gallery
Only problem: The escaped PackageID is now also displayed in the mail subject - but I guess this is a very rare case and with this PR the link and the ID is correct in the mail body.